Skip to content

fix: set taxes and totals before validating subcontracting transaction#3782

Merged
vorasmit merged 3 commits intoresilient-tech:developfrom
karm1000:set-values-even-if-not-eligible
Nov 11, 2025
Merged

fix: set taxes and totals before validating subcontracting transaction#3782
vorasmit merged 3 commits intoresilient-tech:developfrom
karm1000:set-values-even-if-not-eligible

Conversation

@karm1000
Copy link
Copy Markdown
Member

@karm1000 karm1000 commented Nov 7, 2025

Fixes: #3777

Summary by CodeRabbit

  • Refactor

    • Tax initialization now runs earlier and only once, preventing duplicate calculations.
  • Bug Fixes / Tests

    • total_taxes now shows 0.0 (not blank) for subcontracting orders with unregistered entities and for material receipt stock entries.
  • Tests

    • Updated ITC-04 export expectations: adjusted an item's reported value from 0.0 to 200.0.

@karm1000 karm1000 requested a review from ljain112 November 7, 2025 09:52
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Nov 7, 2025

Walkthrough

Moves tax initialization to the start of subcontracting transaction validation, removes a duplicate tax initialization, and updates tests and ITC-04 test data to reflect adjusted numeric totals and txval values.

Changes

Cohort / File(s) Summary
Subcontracting validation reorder
india_compliance/gst_india/overrides/subcontracting_transaction.py
Selected field_map and invoked CustomTaxController(doc, field_map).set_taxes_and_totals() at the start of validate(doc, method=None); removed the duplicate call later in the same function.
Test expectation updates
india_compliance/gst_india/overrides/test_subcontracting_transaction.py
Updated tests to expect total_taxes == 0.0 instead of None for unregistered company subcontracting order and for stock entry material receipt scenarios.
ITC-04 test data adjustment
india_compliance/gst_india/utils/itc_04/test_itc_04_export.py
Adjusted test fixture expected txval for the SRM item under m2jw from 0.0 to 200.0, updating assertions against generated ITC-04 JSON payload.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as validate(doc)
    participant FieldMap as field_map selection
    participant TaxCtrl as CustomTaxController
    participant Remaining as remaining validation logic

    Note over Caller,FieldMap: Reordered — taxes set early
    Caller->>FieldMap: determine field_map
    FieldMap->>TaxCtrl: CustomTaxController(doc, field_map)
    TaxCtrl->>TaxCtrl: set_taxes_and_totals()
    TaxCtrl-->>Caller: taxes & totals set
    Caller->>Remaining: continue validate (checks, updates, return)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check validate() for assumptions about taxes being set later in the function.
  • Verify updated tests cover edge cases (unregistered company, material receipt).
  • Confirm the ITC-04 txval change matches source fixture intent.

Suggested labels

backport version-15-hotfix

Suggested reviewers

  • ljain112

Poem

🐰 I hopped in early, set taxes with care,

removed the twin step that lingered there.
Tests now hum zero, numbers neat and true,
a carrot cheer for validation anew. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: moving tax initialization to occur before validation in the subcontracting transaction logic.
Linked Issues check ✅ Passed Changes in validate() now execute tax initialization upfront before validation, and updated test expectations reflect 0.0 for total_taxes, addressing the objective in #3777 to update tax fields in backend even when ineligible.
Out of Scope Changes check ✅ Passed All changes are directly related to the objective: restructuring tax initialization in subcontracting validation and updating test data/expectations to match the new tax calculation behavior.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8884f56 and 9df3998.

📒 Files selected for processing (1)
  • india_compliance/gst_india/utils/itc_04/test_itc_04_export.py (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-25T08:19:02.607Z
Learnt from: karm1000
Repo: resilient-tech/india-compliance PR: 3354
File: india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py:278-281
Timestamp: 2025-06-25T08:19:02.607Z
Learning: In the Summary of ITC Availed report (india_compliance/gst_india/report/summary_of_itc_availed/summary_of_itc_availed.py), the summary dictionaries created by get_initial_summary() are never empty - they always contain tax field keys (igst_amount, cgst_amount, sgst_amount, cess_amount) with 0 values, so checking for empty dictionaries is unnecessary.

Applied to files:

  • india_compliance/gst_india/utils/itc_04/test_itc_04_export.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Python Unit Tests
  • GitHub Check: Mergify Merge Protections
🔇 Additional comments (1)
india_compliance/gst_india/utils/itc_04/test_itc_04_export.py (1)

58-58: LGTM! Test data correctly updated to reflect the fix.

The change from 0.0 to 200.0 for the txval field correctly reflects the new behavior where taxes and totals are now calculated even for transactions that may not have been considered eligible before. This aligns with the PR objective to ensure backend updates of grand total and tax fields in stock entries.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Comment thread india_compliance/gst_india/overrides/subcontracting_transaction.py Outdated
@karm1000 karm1000 force-pushed the set-values-even-if-not-eligible branch from 9d5e3d0 to 7cea4ab Compare November 7, 2025 11:41
@karm1000 karm1000 requested a review from ljain112 November 7, 2025 11:42
@ljain112
Copy link
Copy Markdown
Member

ljain112 commented Nov 7, 2025

@karm1000 resolve test cases

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.59%. Comparing base (c312d10) to head (9df3998).
⚠️ Report is 182 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3782   +/-   ##
========================================
  Coverage    69.59%   69.59%           
========================================
  Files          182      182           
  Lines        17967    17967           
========================================
  Hits         12504    12504           
  Misses        5463     5463           
Files with missing lines Coverage Δ
.../gst_india/overrides/subcontracting_transaction.py 84.78% <100.00%> (ø)
...india/overrides/test_subcontracting_transaction.py 97.33% <100.00%> (ø)
...iance/gst_india/utils/itc_04/test_itc_04_export.py 100.00% <ø> (ø)

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vorasmit vorasmit merged commit 1f2d9f7 into resilient-tech:develop Nov 11, 2025
13 checks passed
@ljain112
Copy link
Copy Markdown
Member

ljain112 commented Feb 6, 2026

@Mergifyio backport version-15-hotfix

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Feb 6, 2026

backport version-15-hotfix

✅ Backports have been created

Details

mergify Bot added a commit that referenced this pull request Feb 6, 2026
…tfix/pr-3782

fix: set taxes and totals before validating subcontracting transaction (backport #3782)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update grand total and taxes fields in stock entry in backend even if transaction is not eligible.

3 participants